Node 16 upgrade and peer dependency cleanup #250

adamstankiewicz · 2022-04-05T18:31:18Z

Description

Moves from Node 12 ➡️ Node 16, as we now rely on NPM workspaces! 🚀
Resolves the majority of our peer dependency issues, at least the ones that block an npm install on Node 16+.
- As a result, most dependencies installed in the root package.json were instead distributed to each individual package as necessary. The tradeoff is we lose having a single source-of-truth of common "global" dependencies like @edx/frontend-platform, but this is OK and was a premature optimization (i.e., we only have 3 packages).
Updates .nvmrc to include 16 🎉
Moves away from relying on lerna bootstrap, which is a command that is a little magical ✨:
- It does a combination of installing packages, creating/updating package-lock.json, and creating symlinks between packages in this monorepo to allow importing by the module name, e.g. import { useEnterpriseConfig } from '@edx/frontend-enterprise-utils';.
- The symlinking behavior was causing discrepancies between local environments and CI, so instead, we will now rely on running npm install ourselves via lerna exec -- npm install to avoid this symlinking wizardry altogether 🧙
- ~~I think this also may be a step towards using NPM workspaces in the future?~~ Adopts NPM workspaces! 😄

Testing

Using @edx/frontend-enterprise-utils via a module.config.js override in frontend-component-header-edx seemingly led to good results (related PR):

Additionally, all 3 packages (@edx/frontend-enterprise-utils, @edx/frontend-enterprise-catalog-search, and @edx/frontend-enterprise-logistration) are used by frontend-app-admin-portal. In a test via module.config.js, the application seems to run no problem 💪

The Other stuff

Merge checklist:

Evaluate how your changes will impact existing consumers (e.g., frontend-app-learner-portal-enterprise, frontend-app-admin-portal, and frontend-app-enterprise-public-catalog). Will consumers safely be able to upgrade to this change without any breaking changes?
Ensure your commit message follows the semantic-release conventional commit message format. If your changes include a breaking change, ensure your commit message is explicitly marked as a BREAKING CHANGE so the NPM package is released as such.
Once CI is passing, verify the package versions that Lerna will increment to in the Github Action CI workflow logs.
- Note: This may be found in the "Preview Updated Versions (dry run)" step in the Github Action CI workflow logs.

Post merge:

Verify Lerna created a release commit (e.g., chore(release): publish) that incremented versions in relevant package.json and CHANGELOG files, and created Git tags for those versions.
Run the Publish from package.json Github Action workflow to publish these new package versions to NPM.
- This may be triggered by clicking the "Run workflow" option for the master branch.
Verify the new package versions were published to NPM (i.e., npm view <package_name> versions --json).
- Note: There may be a slight delay between when the workflow finished and when NPM reports the package version as being published. If it doesn't appear right away in the above command, try again in a few minutes.

codecov · 2022-04-06T01:56:06Z

Codecov Report

Merging #250 (465f67f) into master (938dfdc) will decrease coverage by 0.40%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #250      +/-   ##
==========================================
- Coverage   76.31%   75.90%   -0.41%     
==========================================
  Files          33       33              
  Lines         591      606      +15     
  Branches      137      152      +15     
==========================================
+ Hits          451      460       +9     
- Misses        126      132       +6     
  Partials       14       14

Impacted Files	Coverage Δ
packages/logistration/src/LoginRedirect.jsx	`100.00% <100.00%> (ø)`
packages/catalog-search/src/FacetListBase.jsx	`71.05% <0.00%> (-5.42%)`	⬇️
packages/catalog-search/src/SearchBox.jsx	`94.64% <0.00%> (-1.66%)`	⬇️
packages/utils/src/learnerPortalLinks.js	`0.00% <0.00%> (ø)`
packages/catalog-search/src/data/utils.js	`100.00% <0.00%> (ø)`
packages/catalog-search/src/SearchContext.jsx	`100.00% <0.00%> (ø)`
packages/catalog-search/src/SearchPagination.jsx	`100.00% <0.00%> (ø)`
packages/catalog-search/src/CurrentRefinements.jsx	`81.57% <0.00%> (+0.49%)`	⬆️
...ages/catalog-search/src/LearningTypeRadioFacet.jsx	`88.88% <0.00%> (+0.65%)`	⬆️
packages/catalog-search/src/SearchHeader.jsx	`91.66% <0.00%> (+0.75%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 938dfdc...465f67f. Read the comment docs.

binodpant · 2022-04-06T11:42:37Z

.github/workflows/ci.yml

+        npm whoami
+    - name: Install and Setup Dependencies
+      run: |
+        npm install


interesting we need to run two install commands?

The first is installing node modules at the root directory (e.g, to install Lerna itself).

The second is to run npm install in all packages. So technically it's 4 installs 🤪

ah I see, makes sense now, thanks!

more the (merrier) 🤣

* Move "globally" installed dependencies from root to each individual package, as needed. * Ensure all peer dependencies include a semver range, on their current versions.

* Updates jest and eslint config to adhere to local package aliases. * Formally adopts NPM workspaces. * Deletes node_modules in the `clean` Makefile targets

adamstankiewicz · 2022-04-13T17:44:42Z

.github/workflows/ci.yml

+    - name: Setup Nodejs
+      uses: actions/setup-node@v3
+      with:
+        node-version: 16


This repository is now relying on NPM workspaces. As such, it is not compatible with Node 12 or Node 14, as NPM workspaces require npm v7+, so we will only run against Node 16.

adamstankiewicz · 2022-04-13T17:46:48Z

.github/workflows/ci.yml

+        git switch -c "actual-branch-not-detached-head"
+        lerna version --allow-branch actual-branch-not-detached-head --no-git-tag-version --no-changelog --no-push --yes


By changing the on from "push" to "pull_request" ensures this workflow is only run when necessary, but in doing so, the checkout is in a detached HEAD state, so we need to force switch to an attached branch for the version dry-run command to work.

BREAKING CHANGE: The Open edX platform is collectively moving towards Node 16. By doing so in this repository, we can now use NPM workspaces in place of Lerna in many places. Lerna is still used for publishing to NPM, updating CHANGELOGs and package.json files upon released. But NPM workspace commands can now be used instead of Lerna commands for the developer experience, which is more performant, easier to reason about, and natively supported by NPM.

macdiesel · 2022-04-14T12:51:29Z

.github/workflows/ci.yml

+        # pulls all commits (needed for lerna / semantic release to correctly version)
+        fetch-depth: 0
+    # pulls all tags (needed for lerna / semantic release to correctly version)
+    - run: git fetch --depth=1 origin +refs/tags/*:refs/tags/*


I'm curious as to why this run directive is the start of it's own sequence and not part of the "Checkout" sequence?

I don't think you can combine run in the same sequence that has a uses. I attempted to move the run directive in the "Checkout" sequence but the CI workflow wasn't running due to an incorrect configuration. I moved it back to its standalone step, but gave it a name, too.

adamstankiewicz force-pushed the astankiewicz/utils-update-only branch from 65dd2b5 to 7b696a3 Compare April 5, 2022 18:32

adamstankiewicz changed the title ~~fix: resolve most peer deps warnings in utils package~~ fix: resolve some peer dependency warnings Apr 5, 2022

long74100 force-pushed the astankiewicz/utils-update-only branch from 8344c30 to f0e21f4 Compare April 5, 2022 21:25

adamstankiewicz changed the title ~~fix: resolve some peer dependency warnings~~ Node 16 upgrade and peer dependency cleanup Apr 6, 2022

Jawayria approved these changes Apr 6, 2022

View reviewed changes

binodpant reviewed Apr 6, 2022

View reviewed changes

binodpant approved these changes Apr 6, 2022

View reviewed changes

long74100 approved these changes Apr 6, 2022

View reviewed changes

This was referenced Apr 7, 2022

build: Added support for node 16 #210

Closed

frontend-enterprise node upgrade openedx/wg-frontend#96

Closed

adamstankiewicz added 17 commits April 13, 2022 10:25

fix: resolve most peer deps warnings in utils package

6c8d611

fix: address peer dependency issues

c0e6df1

* Move "globally" installed dependencies from root to each individual package, as needed. * Ensure all peer dependencies include a semver range, on their current versions.

fix: moar updates

3fb052c

build: update ci.yml github action workflow

ebab6bb

build: use node 16 to install deps

d73fa11

build: use node matrix in ci

5a8cbf8

build: adds NPM workspaces to package.json

95c879e

chore: upgrade @edx/frontend-platform devDeps

036ae9e

fix: update package-lock.json

5317170

fix: regenerate package-lock.json files

3c68baf

build: remove workspaces from package.json

ef00ec9

chore: move prop types to devDeps

8763515

chore: move prop-types back to deps due to eslint error

3680827

build: use npm i vs npm ci

e8d7c89

fix: push up local changes

8a8dcf9

fix: add extendJestConfig helper function

851a12f

fix: make things work

80c581e

* Updates jest and eslint config to adhere to local package aliases. * Formally adopts NPM workspaces. * Deletes node_modules in the `clean` Makefile targets

adamstankiewicz force-pushed the astankiewicz/utils-update-only branch from c5e4a19 to 80c581e Compare April 13, 2022 16:59

build: only run CI in Node 16

77941a0

adamstankiewicz added 7 commits April 13, 2022 13:12

build: run CI workflow on pull_request actions

8b9be68

build: only install npm packages at root with workspaces

9e05565

build: only install npm packages at root with workspaces pt2

3710f45

build: update how branch name is retrieved in version dry run

b20474a

build: only install npm packages at root with workspaces pt2 again

7bd6dce

build: try to fix detached head issue

8200d01

build: try to fix detached head issue pt2

3d5a36e

adamstankiewicz commented Apr 13, 2022

View reviewed changes

adamstankiewicz added 4 commits April 13, 2022 13:57

build: remove eslint-import-resolver-alias

6198016

build: run workspaces command using npm

3c517b3

build: update npm scripts to use npm workspaces and update readme

ea3cfb3

macdiesel reviewed Apr 14, 2022

View reviewed changes

macdiesel approved these changes Apr 14, 2022

View reviewed changes

adamstankiewicz added 2 commits April 14, 2022 09:51

build: attempt to fetch tags in checkout workflow step

08b7563

build: move back to pulling tags as separate step

465f67f

adamstankiewicz merged commit d4e3caf into master Apr 14, 2022

adamstankiewicz deleted the astankiewicz/utils-update-only branch April 14, 2022 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node 16 upgrade and peer dependency cleanup #250

Node 16 upgrade and peer dependency cleanup #250

adamstankiewicz commented Apr 5, 2022 •

edited

Loading

codecov bot commented Apr 6, 2022 •

edited

Loading

binodpant Apr 6, 2022

adamstankiewicz Apr 6, 2022

binodpant Apr 6, 2022

binodpant Apr 6, 2022

adamstankiewicz Apr 13, 2022 •

edited

Loading

adamstankiewicz Apr 13, 2022

macdiesel Apr 14, 2022

adamstankiewicz Apr 14, 2022

		git switch -c "actual-branch-not-detached-head"
		lerna version --allow-branch actual-branch-not-detached-head --no-git-tag-version --no-changelog --no-push --yes

Node 16 upgrade and peer dependency cleanup #250

Node 16 upgrade and peer dependency cleanup #250

Conversation

adamstankiewicz commented Apr 5, 2022 • edited Loading

Description

Testing

The Other stuff

codecov bot commented Apr 6, 2022 • edited Loading

Codecov Report

binodpant Apr 6, 2022

Choose a reason for hiding this comment

adamstankiewicz Apr 6, 2022

Choose a reason for hiding this comment

binodpant Apr 6, 2022

Choose a reason for hiding this comment

binodpant Apr 6, 2022

Choose a reason for hiding this comment

adamstankiewicz Apr 13, 2022 • edited Loading

Choose a reason for hiding this comment

adamstankiewicz Apr 13, 2022

Choose a reason for hiding this comment

macdiesel Apr 14, 2022

Choose a reason for hiding this comment

adamstankiewicz Apr 14, 2022

Choose a reason for hiding this comment

adamstankiewicz commented Apr 5, 2022 •

edited

Loading

codecov bot commented Apr 6, 2022 •

edited

Loading

adamstankiewicz Apr 13, 2022 •

edited

Loading